Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(meta): refresh metastore if there is zdb up/down #129

Merged
merged 1 commit into from
Nov 19, 2024
Merged

Conversation

iwanbk
Copy link
Member

@iwanbk iwanbk commented Nov 18, 2024

part of #127 :

when the meta backend up, zstor monitor automatically reconnect to that backend

@iwanbk iwanbk requested a review from LeeSmet as a code owner November 18, 2024 08:29
@iwanbk iwanbk force-pushed the meta-refresh branch 3 times, most recently from 92e051e to a995b37 Compare November 18, 2024 08:43
@iwanbk iwanbk force-pushed the meta-refresh branch 2 times, most recently from ce53683 to b4018ec Compare November 18, 2024 09:33
@iwanbk
Copy link
Member Author

iwanbk commented Nov 19, 2024

I got some issue during encryption on

let plaintext = cipher.decrypt(nonce, &data[AES_GCM_NONCE_SIZE..]).unwrap();

but also happened on the master (before all of my changes), but can't reproduce it again.

I'm checking it now.

@iwanbk iwanbk marked this pull request as draft November 19, 2024 07:34
@iwanbk
Copy link
Member Author

iwanbk commented Nov 19, 2024

@LeeSmet

Do you aware of some issue with

let plaintext = cipher.decrypt(nonce, &data[AES_GCM_NONCE_SIZE..]).unwrap();
?

@iwanbk
Copy link
Member Author

iwanbk commented Nov 19, 2024

OK, i found the issue, use the wrong config for the encryption

$ git diff
diff --git a/zstor/src/actors/backends.rs b/zstor/src/actors/backends.rs
index 6dd68eb..d9d2929 100644
--- a/zstor/src/actors/backends.rs
+++ b/zstor/src/actors/backends.rs
@@ -400,7 +400,7 @@ impl Handler<CheckBackends> for BackendManagerActor {
                         };
                         let Meta::Zdb(meta_config) = config.meta();
                         let encoder = meta_config.encoder();
-                        let encryptor = match meta_config.encryption() {
+                        let encryptor = match config.encryption() {
                             Encryption::Aes(key) => AesGcm::new(key.clone()),
                         };
                         let new_cluster = ZdbMetaStore::new(

@iwanbk iwanbk marked this pull request as ready for review November 19, 2024 08:37
@iwanbk
Copy link
Member Author

iwanbk commented Nov 19, 2024

i just found the new_metastore func, but it is not used

pub async fn new_metastore(

I tried to use it as well, but it doesn't meet usecase.
If i adjust it, looks like it has to be moved zdb_meta.rs.

If you like, i could do another PR to refactor that code, along with another improvements.
I found that many closures are too long that made it harder to understand

@LeeSmet
Copy link
Contributor

LeeSmet commented Nov 19, 2024

i just found the new_metastore func, but it is not used

pub async fn new_metastore(

I tried to use it as well, but it doesn't meet usecase. If i adjust it, looks like it has to be moved zdb_meta.rs.

If you like, i could do another PR to refactor that code, along with another improvements. I found that many closures are too long that made it harder to understand

Yes, feel free to do so if it is an improvement to the code

@LeeSmet LeeSmet merged commit 78e926d into master Nov 19, 2024
4 checks passed
@LeeSmet LeeSmet deleted the meta-refresh branch November 19, 2024 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants